Fix false positive curation for article links with extra path components #1235
Conversation
|
@swift-ci please test |
| case 0,1: | ||
| return NodeURLGenerator.Path.article(bundleName: bundle.displayName, articleName: path).stringValue | ||
| case 2: | ||
| return NodeURLGenerator.Path.documentationFolder + path |
There was a problem hiding this comment.
| return NodeURLGenerator.Path.documentationFolder + path | |
| return NodeURLGenerator.Path.documentationFolder + "/" + path |
Do we need to add a separator here to construct the article's URL properly? Running the new unit test I'm seeing this value for sourceArticlePath below: /documentationWrongModuleName/First. Looks like this would never match, even if the path was correct?
Or maybe I'm missing something? I would expect other unit tests to fail if we really needed the slash here, assuming we test article curation extensively already.
There was a problem hiding this comment.
I think you're right that there should be a slash here. However, I couldn't find any test that reaches this code where the link isn't also expected to fail to resolve. I also didn't figure out how to write a new test that reaches this code when the link isn't expected to fail to resolve.
The pattern that we're trying to identify here is CatalogName/ArticleName but that link would have resolved earlier in this method, so it reach this code. Only when the catalog/module name is wrong do we reach this code but that link is expected to fail to resolve regardless. Ideally we'd be able to remove this entire fallback code path but that requires larger changes more suitable for a different PR.
|
@swift-ci please test |
|
@swift-ci please test |
…nts (swiftlang#1235) * Fix false positive curation for article links with extra path components rdar://150874238 * Remove explicit `.init` in test file * Remove unused variable in test * Add missing path separator in curator fallback code path
This change addresses the issue where DocC internally checks for single root pages but doesn't surface warnings to developers when multiple roots are detected.
Added two new warnings:
1. Multiple @TechnologyRoot directives (org.swift.docc.MultipleTechnologyRoots)
- Warns when multiple articles have the @TechnologyRoot directive
- Provides source locations and fix-it suggestions to remove extra directives
2. Multiple main modules (org.swift.docc.MultipleMainModules)
- Warns when symbol graphs contain multiple main modules
- Lists all detected module names in the warning message
- Suggests documenting each module separately
Both warnings are non-blocking - documentation still builds successfully, allowing developers to address the issues at their discretion.
Added comprehensive tests for both warning scenarios to ensure correct diagnostic emission and that documentation continues to build properly.
Resolves: swiftlang#1235
A few additional notes about the implementation:
1. Non-blocking warnings: Both warnings are intentionally non-blocking (severity: .warning) so existing documentation builds won't break. This gives developers time to fix their setups.
2. Source locations: For @TechnologyRoot warnings, each warning includes the exact source location of the directive, making it easy for developers to find and fix. For multiple modules, there's no specific source location since it's about the overall symbol
graph structure.
3. Backwards compatibility: The changes don't affect any existing behavior - they only add new warnings that help developers identify potential issues with their documentation structure.
4. Testing approach: The tests create minimal reproducible cases and verify both that warnings are emitted and that the documentation still functions correctly despite the warnings.
Bug/issue #, if applicable: rdar://150874238
Summary
This fixes a false positive curation for article links with extra path components.
Because the
DocumentationCuratorwould previously only look at the last path component for articles, a link like<doc:nonsense/nonsense/nonsense/RealArticleName>would result in an association in the topic graph betweendoc://catalog.identifier/documentation/CatalogName/RealArticleNameand the page that contains the curating topic section, even though DocC would fail to resolve the link (meaning that the page won't render that link in that topic section.Dependencies
None.
Testing
Add extra nonsense path components to an article link in any topic section on any page (for example like in this test). That unresolved link shouldn't do anything.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded[ ] Updated documentation if necessary